-
Notifications
You must be signed in to change notification settings - Fork 1.1k
statement-store: implement new rpc api #10690
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: master
Are you sure you want to change the base?
Conversation
…e_dev --bump major'
Co-authored-by: Alexandru Gheorghe <[email protected]>
Signed-off-by: Alexandru Gheorghe <[email protected]>
Signed-off-by: Alexandru Gheorghe <[email protected]>
Signed-off-by: Alexandru Gheorghe <[email protected]>
Signed-off-by: Alexandru Gheorghe <[email protected]>
Signed-off-by: Alexandru Gheorghe <[email protected]>
Signed-off-by: Alexandru Gheorghe <[email protected]>
Signed-off-by: Alexandru Gheorghe <[email protected]>
Signed-off-by: Alexandru Gheorghe <[email protected]>
Signed-off-by: Alexandru Gheorghe <[email protected]>
Signed-off-by: Alexandru Gheorghe <[email protected]>
Signed-off-by: Alexandru Gheorghe <[email protected]>
Signed-off-by: Alexandru Gheorghe <[email protected]>
Signed-off-by: Alexandru Gheorghe <[email protected]>
Signed-off-by: Alexandru Gheorghe <[email protected]>
Signed-off-by: Alexandru Gheorghe <[email protected]>
Signed-off-by: Alexandru Gheorghe <[email protected]>
Signed-off-by: Alexandru Gheorghe <[email protected]>
Signed-off-by: Alexandru Gheorghe <[email protected]>
Signed-off-by: Alexandru Gheorghe <[email protected]>
Signed-off-by: Alexandru Gheorghe <[email protected]>
|
All GitHub workflows were cancelled due to failure one of the required jobs. |
Signed-off-by: Alexandru Gheorghe <[email protected]>
AndreiEres
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Great work, Alex!
Looks good, left a few questions.
s0me0ne-unkn0wn
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm just skimming it for now. The API itself looks good and quite intuitive. I'm yet to understand the mechanics happening inside.
Signed-off-by: Alexandru Gheorghe <[email protected]>
| log::info!("Statement store test passed"); | ||
| log::info!("Keeping network alive"); | ||
|
|
||
| tokio::time::sleep(Duration::from_hours(24)).await; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
why we need hang the test for 24h?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It is there for App guys to have a node setup they can run locally, will remove it before merge.
| fn try_into(self) -> Result<CheckedTopicFilter> { | ||
| match self { | ||
| TopicFilter::Any => Ok(CheckedTopicFilter::Any), | ||
| TopicFilter::MatchAll(topics) => { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should we check topics.len() <= 4?
| let mut evicted = HashSet::new(); | ||
| let mut would_free_size = 0; | ||
| let priority = Priority(statement.priority().unwrap_or(0)); | ||
| let expiry = Expiry(statement.expiry()); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
why not statement.expiry() -> Expiry?
Work in progress
Description
Implements the proposed changes from here #10452, where the API surface has been limited to two functions
submitand asubscribe_statement.Submit changes
For submitting the only changes are implementing the logic of the newly
expiryfield where statements with an expiration timestamp lower than current time get rejected.Subscribe changes
For subscription the approach is to have a configurable number of workers that keep track of all subscriptions, each time a new subscription arrives it gets assigned by a round-robin protocol to one of the workers.
When a new statements gets accepted by the statement-store it notifies all workers about the statement, the workers then go and evaluate all subscription filters they are assigned to and notifies each of them accordingly.
Remaining work
statement_store_bench.rsto using subscribe API, ported and performance stay the same on the benchmark, because bottlenecks are elsewhere and being addressed.Local testing
To obtain a local node with the new APIs available run:
The rpc addresses are visible in the output: